Skip to content

feat(rig): provision a rig-managed .gitignore block at init/apply#23

Closed
alex-mextner wants to merge 2 commits into
mainfrom
rig-gitignore
Closed

feat(rig): provision a rig-managed .gitignore block at init/apply#23
alex-mextner wants to merge 2 commits into
mainfrom
rig-gitignore

Conversation

@alex-mextner

Copy link
Copy Markdown
Owner

Summary

Adds two OpenAI-compatible keyed HTTP backends to the review CLI:

  • z.ai (Zhipu / GLM)zai / z.ai / zhipu / glm, plus zai:<model> (default model glm-4.6, base https://api.z.ai/api/paas/v4). Key from ZAI_API_KEY / ZHIPU_API_KEY.
  • common-codecommon-code / commoncode / common_code, plus common-code:<model>. Modelled as a keyed OpenAI-compatible HTTP backend (commandcode / DeepSeek family). Default base https://api.deepseek.com + model deepseek-chat, both overridable via COMMON_CODE_BASE_URL / COMMON_CODE_MODEL. Key resolves from COMMON_CODE_API_KEY / COMMANDCODE_API_KEY / DEEPSEEK_API_KEY. No common-code CLI exists on PATH, so it is intentionally an HTTP backend, not a CLI wrapper — DeepSeek is the default but everything is env-overridable.

Both share a single _openai_compatible_request helper (same wire shape) and a _resolve_key multi-name resolver that also reads ~/.config/review-cli/.env / GEMINI_ENV_FILE. Routing added in resolve_backend; backend_available checks for keys.

Files

  • reviewlib/backends.py (+141/-3) — backends, key resolution, routing
  • reviewlib/config.py, reviewlib/__init__.py — wiring/exports
  • tests/test_provider_keys.py (+389) — full unit coverage
  • tests/smoke.sh — provider-keys test added to smoke suite

Tests

  • tests/test_provider_keys.py — all PASS (model-suffix parsing, env/file precedence, OpenAI request shape, requested-vs-provider model id, HTTP error mapping, backend_available key reflection).
  • Full tests/smoke.sh — rc=0 (includes visual suite, which ran with ImageMagick + Pillow present).

Rebased onto current main (f3d17f8). No overlap with the already-merged visual/decompose work.

🤖 Generated with Claude Code

Machine-artifact ignores must be provisioned by rig, not hand-edited into
~/.gitignore. rig now maintains a marker-delimited block in the repo's
.gitignore so harness artifacts are ignored declaratively, reconciled like
every other category.

- Default entry: .claude/worktrees/ (Claude Code's throwaway worktrees).
  .serena/ is intentionally NOT ignored — Serena state is committed shared
  project memory.
- Marker-fenced block (BEGIN/END); rig edits only its own lines, every other
  line preserved byte-for-byte (CRLF, no-final-newline, trailing blanks via
  offset splicing, not splitlines/rejoin).
- Idempotent: a correct block is a no-op; a missing block is appended
  (creating .gitignore if absent); a drifted block is replaced in place.
- States shared by apply + drift via resolve_gitignore: ok / create / update /
  conflict (unbalanced markers, left untouched + surfaced) / io_error
  (unreadable path → error, never a silent skip).
- Config: gitignore.{enabled,entries}; default ON, opt out with enabled:false.
  Fail-closed validation rejects a non-mapping block, non-bool enabled, unknown
  keys, non-string entries, and an entry colliding with a managed marker.
- rig status reports drift on the block, including a leftover block after the
  category is disabled (check_disabled_gitignore, mirrors the dispatcher scan).
- Tests cover every state, idempotency, verbatim preservation, drift parity,
  and config validation. Docs (config-schema.md) and the dogfooded rig.yaml
  updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26cb7dc843

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread riglib/actions/runner.py
Comment on lines +1354 to +1356
with path.open(encoding="utf-8", newline="") as fh:
content = fh.read()
except OSError as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle non-UTF-8 .gitignore contents

When a repo's .gitignore contains any byte that is not valid UTF-8, this strict text read raises UnicodeDecodeError, but the handler only catches OSError. rig status reaches this through drift._check_gitignore without run_plan's broad exception wrapper, so those repos crash with a traceback instead of surfacing the documented io_error drift item; catch decode errors here or read with a lossless error strategy.

Useful? React with 👍 / 👎.

…tignore imports

Make the disabled-gitignore drift check detect rig's block the same way apply
does, and source the gitignore constants from their home module.

- drift.check_disabled_gitignore now matches the begin marker via
  _find_marker_lines (the offset scanner resolve_gitignore already uses) read
  with newline="" instead of an ad-hoc splitlines()/.strip() pass — so drift
  detection can never diverge from apply (CRLF / whitespace-padded marker lines
  match identically; _find_marker_lines returns an empty list when absent, so
  the truthiness check is correct).
- Import GITIGNORE_BEGIN/END_MARKER and GITIGNORE_DEFAULT_ENTRIES from
  riglib.config (their canonical schema-layer home) in drift, plan, and tests,
  instead of re-exporting through runner; lift plan's deferred import to module
  top (no import cycle — config already imported there). runner's header comment
  updated to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alex-mextner added a commit that referenced this pull request Jun 17, 2026
…sfile

Rebuild #23's gitignore feature under the CTO-chosen GLOBAL design: rig owns
ONE marker-delimited block in git's global core.excludesfile so the harness's
throwaway **/.claude/worktrees/ is ignored in EVERY repo on the machine, with
zero per-repo commits and no per-repo `rig apply`.

Wired like the git-hooks dispatcher (a `git config --global` setting + a
managed file), in the GLOBAL rig layer. Target resolution at apply time:
honor an existing core.excludesfile; when unset, set it to ~/.config/git/ignore
AND write the block there (so a clean machine is fully provisioned by `rig
init` alone). Reuses #23's marker reconcile machine (create/update/ok/conflict/
io_error), retargeted, plus:
  - strict idempotency: a re-apply is a byte-identical no-op;
  - dedup of the managed region: several duplicated blocks collapse to one,
    preserving user content BETWEEN blocks (splice per marker-pair, not
    first-begin..last-end);
  - drift in the GLOBAL section flags a missing/divergent/duplicated block and
    an unset core.excludesfile rig would set.

Default ON at plan level (an absent `gitignore` key still provisions), so the
GLOBAL block is NOT scaffolded into committed repo rig.yaml. The canonical
block text (markers + fixed comment + entry) is byte-identical to what a
provisioned machine already has, making re-apply zero-churn.

Review: ran multi-model `review` on this diff; addressed findings (interleaved
user-line preservation, XDG isolation in tests, scaffold cleanup, doc fixes).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alex-mextner added a commit that referenced this pull request Jun 17, 2026
…sfile (supersedes #23) (#29)

* feat(rig): provision .claude/worktrees ignore via global core.excludesfile

Rebuild #23's gitignore feature under the CTO-chosen GLOBAL design: rig owns
ONE marker-delimited block in git's global core.excludesfile so the harness's
throwaway **/.claude/worktrees/ is ignored in EVERY repo on the machine, with
zero per-repo commits and no per-repo `rig apply`.

Wired like the git-hooks dispatcher (a `git config --global` setting + a
managed file), in the GLOBAL rig layer. Target resolution at apply time:
honor an existing core.excludesfile; when unset, set it to ~/.config/git/ignore
AND write the block there (so a clean machine is fully provisioned by `rig
init` alone). Reuses #23's marker reconcile machine (create/update/ok/conflict/
io_error), retargeted, plus:
  - strict idempotency: a re-apply is a byte-identical no-op;
  - dedup of the managed region: several duplicated blocks collapse to one,
    preserving user content BETWEEN blocks (splice per marker-pair, not
    first-begin..last-end);
  - drift in the GLOBAL section flags a missing/divergent/duplicated block and
    an unset core.excludesfile rig would set.

Default ON at plan level (an absent `gitignore` key still provisions), so the
GLOBAL block is NOT scaffolded into committed repo rig.yaml. The canonical
block text (markers + fixed comment + entry) is byte-identical to what a
provisioned machine already has, making re-apply zero-churn.

Review: ran multi-model `review` on this diff; addressed findings (interleaved
user-line preservation, XDG isolation in tests, scaffold cleanup, doc fixes).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(rig): global-excludes unit tests, smoke leg, schema docs

- tests/test_global_excludes.py: target resolution both ways (core.excludesfile
  set vs unset, with injected git-config seams so no test runs real `git config
  --global`), every reconcile state, STRICT byte-identical re-apply, the
  dedup-of-managed-region collapse (incl. preserving a user line between blocks),
  CRLF/trailing-blank/no-final-newline preservation, drift parity, disabled-but-
  installed leftover scan, and a byte-stable canonical-block regression pin.
- conftest: autouse fixtures isolate HOME + XDG_CONFIG_HOME and stub the
  git-config read/write seams suite-wide, so a full-plan e2e test can never
  touch the real ~/.gitignore or real global git config.
- smoke.sh: HOME-isolated leg exercises the real init/apply flow — asserts
  core.excludesfile is set to the XDG default, the block is written, and the
  block is byte-stable (markers=2) across a re-apply. Also makes the mcp leg
  conditional on the carrier and prefers `uv run pytest`.
- docs/config-schema.md: the `gitignore` (global core.excludesfile) section and
  validation note.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@alex-mextner

Copy link
Copy Markdown
Owner Author

Superseded by #29 (global core.excludesfile approach chosen over per-repo .gitignore block). Closing as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant